Skip to content

chore(gmpctl): remove install of bingo deps - #1963

Closed
bernot-dev wants to merge 1 commit into
mainfrom
remove-gmpctl-bingo-install
Closed

chore(gmpctl): remove install of bingo deps#1963
bernot-dev wants to merge 1 commit into
mainfrom
remove-gmpctl-bingo-install

Conversation

@bernot-dev

Copy link
Copy Markdown
Collaborator

#1861 removed Bingo. This removes the dependency on Bingo from gmpctl.

@bernot-dev
bernot-dev requested a review from bwplotka June 18, 2026 19:24
@bernot-dev bernot-dev self-assigned this Jun 18, 2026
@bernot-dev
bernot-dev marked this pull request as ready for review June 18, 2026 19:25

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes the execution of the presubmit script from the release-lib::manifests_regen function in ops/gmpctl/lib.sh. Feedback indicates that this change completely disables manifest regeneration, and a suggestion has been provided to retain the invocation of hack/presubmit.sh without the obsolete bingo variable swap.

I am having trouble creating individual review comments. Click here to see my feedback.

ops/gmpctl/lib.sh (450-455)

critical

By removing the execution of hack/presubmit.sh manifests, the release-lib::manifests_regen function no longer actually regenerates the manifests. It only prints a success message without performing any work.

Since hack/presubmit.sh now uses go tool with tools/go.mod to run helm, yq, and addlicense, we no longer need the bingo variable swap, but we still must invoke the presubmit script to perform the regeneration.

	bash "${dir}/hack/presubmit.sh" manifests

@bernot-dev
bernot-dev force-pushed the remove-gmpctl-bingo-install branch from afd8d74 to 06140d3 Compare June 18, 2026 19:26
@bernot-dev
bernot-dev force-pushed the remove-gmpctl-bingo-install branch from 06140d3 to 1ca1363 Compare June 18, 2026 19:29
@bernot-dev

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds gmpctl to the list of policies in .conform.yaml and simplifies the manifest regeneration process in ops/gmpctl/lib.sh by removing manual tool installations (helm, addlicense, yq) and a hacky bingo variable swap, delegating the execution directly to the presubmit script. There are no review comments, and I have no feedback to provide.

Comment thread ops/gmpctl/lib.sh
echo "#!/bin/bash" >"${dir}/.bingo/variables.env" # Clean the file.
YQ="$(which yq)" HELM="$(which helm)" ADDLICENSE="$(which addlicense)" bash "${dir}/hack/presubmit.sh" manifests
cp "${dir}/.bingo/variables.env.bak" "${dir}/.bingo/variables.env"
bash "${dir}/hack/presubmit.sh" manifests

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this won't work across all versions - it only makes it work for 0.19/main.

This could make it work for now: https://github.com/GoogleCloudPlatform/prometheus-engine/pull/1938/changes#r3441808186

@bernot-dev bernot-dev closed this Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants